fix(rpc): avoid race when reading storage proof state - #4064

Claude finished @cicr99's task in 12m 42s —— View job

Review summary

Blocker (1 issue, posted inline on both rpc/v10/storage.go:209 and rpc/v9/storage.go:96):

Swapping HeadState() for h.bcReader.StateAtBlockNumber(chainHeight) fixes the described race, but StateAtBlockNumber returns a stateHistory-backed reader (core/state/history.go) whose ClassTrie(), ContractTrie(), and ContractStorageTrie() are all stubbed to unconditionally return ErrHistoricalTrieNotSupported. StorageProof needs exactly these methods to build the Merkle proofs, so every real (non-mocked) call to starknet_getStorageProof now fails with an internal error.

This isn't theoretical — this PR's own CI run already shows it: Juno Test (run 34593901648) fails on TestStorageProof_StorageRoots/get_contract_proof in both rpc/v9 and rpc/v10, which exercise a real blockchain.Blockchain + synced state rather than a mock. The unit tests added/touched in this PR pass only because they mock bcReader.StateAtBlockNumber to return a MockStateReader that doesn't have this historical-trie restriction, so the mocked tests mask the regression.

The underlying race is legitimate and worth fixing, but the fix needs a state accessor that pins to a specific block's global state root while still supporting trie reconstruction — e.g., resolving the state root at chainHeight directly and building a state.NewStateReader(stateRoot, ...), or extending the history state path to support ClassTrie/ContractTrie/ContractStorageTrie for the head block. As currently written, this PR would need to fail CI and shouldn't be merged in its current form.

Review in progress

• Gather context, read prior review feedback (the earlier StateAtBlockNumber regression was addressed by adding a new TrieStateAtBlockNumber method)
• Review blockchain/blockchain.go, blockchain/statebackend/{statebackend,deprecated,types}.go
• Review rpc/v9/storage.go, rpc/v10/storage.go handler changes
• Review test/mocks updates
• Post findings

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

func (b *deprecatedStateBackend) TrieStateAtBlockNumber(
	blockNumber uint64,
) (core.StateReader, StateCloser, error) {
	if err := pruner.RequireStateRetainedByBlockNumber(
		b.database,
		b.retentionFloor,
		blockNumber,
	); err != nil {
		return nil, nil, err
	}

	//nolint:staticcheck,nolintlint // used by old state
	txn := b.database.NewIndexedBatch()
	height, err := core.GetChainHeight(txn)
	if err != nil {
		return nil, nil, err
	}
	if height != blockNumber {
		return nil, nil, db.ErrKeyNotFound
	}

	return deprecatedstate.New(txn), NoopStateCloser, nil
}

This new solution adds a trie-capable state accessor for storage proofs:
For the new state implementation, it opens a regular StateReader from the retained state root for the requested block number, so it fixes the inconsistency properly.
However, for deprecated state, it is still not fully race-free. It checks that the deprecated trie reader is being opened while the DB still reports the same head height. If the chain already moved, it fails instead of knowingly returning the wrong state. This is still better than before, as it reduces the race window, but it's still not the optimal solution.

For full correctness we'd still need the DB snapshots solution

Codecov Report

❌ Patch coverage is 82.14286% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.34%. Comparing base (64e105c) to head (97dc33b).
⚠️ Report is 6 commits behind head on main.

@@            Coverage Diff             @@
##             main    #4064      +/-   ##
==========================================
- Coverage   79.47%   79.34%   -0.13%     
==========================================
  Files         466      466              
  Lines       36015    36035      +20     
==========================================
- Hits        28623    28593      -30     
- Misses       7383     7433      +50     
  Partials        9        9              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.